-
Notifications
You must be signed in to change notification settings - Fork 25.5k
[ESQL] Support datetime data type in Least and Greatest functions #113961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ESQL] Support datetime data type in Least and Greatest functions #113961
Conversation
Documentation preview: |
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Hi @not-napoleon, I've created a changelog YAML for you. |
@elasticmachine update branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I think we're ok not writing another CSV test for this.
I could see a world where we try to build CSV-style tests from our unit test scenarios. But that's for later i think.
You did write a CSV test and I scrolled by. Sorry! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💔 Backport failed
You can use sqren/backport to manually backport by running |
…astic#113961) While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it. It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now. --------- Co-authored-by: Elastic Machine <[email protected]>
…astic#113961) While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it. It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now. --------- Co-authored-by: Elastic Machine <[email protected]>
…13961) (#114130) While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it. It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now. --------- Co-authored-by: Elastic Machine <[email protected]>
…13961) (#114138) While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it. It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now. --------- Co-authored-by: Elastic Machine <[email protected]>
…astic#113961) While working on Date Nanos, I noticed that Least and Greatest didn't have support for datetime. This PR corrects that and adds tests for it. It seems to me that resolveType() is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now. --------- Co-authored-by: Elastic Machine <[email protected]>
Resolves #109998 For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in #113961. I've added CSV tests and unit tests for all the functions listed in the original ticket. --------- Co-authored-by: Elastic Machine <[email protected]>
…114056) Resolves elastic#109998 For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket. --------- Co-authored-by: Elastic Machine <[email protected]>
…14056) (#115351) * [ESQL] Support date_nanos on functions that take "any" type (#114056) Resolves #109998 For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in #113961. I've added CSV tests and unit tests for all the functions listed in the original ticket. --------- Co-authored-by: Elastic Machine <[email protected]> * Mute failing watcher test Cherry-pick f8e931d#diff-41386766c394f14f5f205f92bb26eb1420b80af0057c78b2842fcc7ddd3d67aaR326 For whatever reason, git cherry-pick is having some difficulty with this, so I just hand copied the mute. * pull in another mute --------- Co-authored-by: Elastic Machine <[email protected]>
…114056) Resolves elastic#109998 For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket. --------- Co-authored-by: Elastic Machine <[email protected]>
…114056) Resolves elastic#109998 For the most part, this is just adding tests. Greater and Least have actual production code changes - notably toEvaluator is modified to map date nanos to the long evaluator. This parallels the work done in elastic#113961. I've added CSV tests and unit tests for all the functions listed in the original ticket. --------- Co-authored-by: Elastic Machine <[email protected]>
While working on Date Nanos, I noticed that Least and Greatest didn't have support for
datetime
. This PR corrects that and adds tests for it.It seems to me that
resolveType()
is doing the wrong thing for these functions, as it accepts types that then do not have evaluator mappings, but refactoring that seems out of scope right now.